-
Notifications
You must be signed in to change notification settings - Fork 326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Suppress needlessly alarming messages #628
base: main
Are you sure you want to change the base?
Conversation
These packages are not marked `REQUIRED` and when this project is used as a dependency of another CMake project they don't need to be findable when this CMakeLists.txt is read. They may in fact be found later in the configuration process, so the messages when they actually _are_ needed, so the messages ``` -- Could NOT find dispatch (missing: dispatch_DIR) -- Could NOT find Foundation (missing: Foundation_DIR) -- Could NOT find XCTest (missing: XCTest_DIR) ``` are needlessly alarming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose that this is fine. The problem is that QUIET
is now applied unconditionally which means that when building the toolchain, we would not see in the logs if a mismatched version of these toolchain vended libraries are accidentally picked up. Perhaps it makes sense to conditionalise this on whether the package is being built as a top-level package?
CC: @etcwilde - Evan may have thoughts on the logging as well.
@swift-ci please test |
Yeah, I'd prefer to keep the logging. If they aren't a |
Maybe instead of marking these |
Maybe you could conditionally
|
@dabrahams the condition that you need I can see an argument for eliding the |
@compnerd I don't understand what your comment means. I wasn't suggesting any heuristic. I don't think there's any reason it should make a difference which version of those libraries are needed for any particular build. Assuming you want to keep the |
The normal usage for the build is with a custom build of the dependencies and therefore the default should be non-QUIET. The special case is Darwin, where these are system provided, so we could exclude the checks there. |
I still don't understand the "therefore." Does using a custom build of the dependencies require that they have been set up by that point? Are they really dependencies at all? |
Yes, the custom build of dependencies requires that they are setup by that point (except on Darwin where they are system provided).
Yes, dispatch is a transitive dependency due to Foundation.
That is a great suggestion! We should conditionalise the |
Co-authored-by: Saleem Abdulrasool <[email protected]>
These packages are not marked
REQUIRED
and when this project is used as a dependency of another CMake project they don't need to be findable when this CMakeLists.txt is read. They may in fact be found later in the configuration process, so the messages when they actually are needed, so the messagesare needlessly alarming.
Replace this paragraph with a description of your changes and rationale. Provide links to an existing issue or external references/discussions, if appropriate.
Checklist